Lokasi ngalangkungan proxy:   [ UP ]  
[Ngawartoskeun bug]   [Panyetelan cookie]                
Skip to content

Fix duplicate DOCS entries when asciidoctor is present#3

Closed
jnasbyupgrade wants to merge 1 commit into
masterfrom
fix-docs-duplicate-html
Closed

Fix duplicate DOCS entries when asciidoctor is present#3
jnasbyupgrade wants to merge 1 commit into
masterfrom
fix-docs-duplicate-html

Conversation

@jnasbyupgrade

@jnasbyupgrade jnasbyupgrade commented May 27, 2026

Copy link
Copy Markdown

Problem

When asciidoctor is installed, make install fails with:

/usr/bin/install: will not overwrite just-created '.../object_reference.html' with './/doc/object_reference.html'

DOCS ends up containing doc/object_reference.html twice:

  1. Line 23 of pgxntool/base.mk: $(wildcard doc/*) picks up the committed .html file
  2. Line 135: DOCS += $(ASCIIDOC_HTML) adds the same path again as a generated asciidoc target

Both paths expand to the same filename, so install receives it twice in one invocation and refuses to overwrite a file it just wrote.

Fix

Use $(filter-out) to remove any ASCIIDOC_HTML files already present in DOCS before appending, then $(sort) to deduplicate. This is the same approach already taken in Postgres-Extensions/pgxntool 2.x.

Reproducer

Any project that commits its generated .html doc alongside its .adoc source, with asciidoctor in PATH.

@claude
Fix duplicate DOCS entries when asciidoctor is present
259e663 
When asciidoctor is installed, DOCS is built by two paths:
- Line 23: $(wildcard doc/*) picks up any committed .html files
- Line 135: ASCIIDOC_HTML adds the generated .html targets

If a .html file is committed alongside its .adoc source (as in
doc/object_reference.html + doc/object_reference.adoc), the file appears
in DOCS twice. This causes `install` to pass the same path to
/usr/bin/install twice in a single invocation, which fails with:

  install: will not overwrite just-created '...html' with '...html'

Fix by using $(filter-out) to remove any ASCIIDOC_HTML entries already
present in DOCS before appending them, then $(sort) to deduplicate.
This matches the approach already taken in Postgres-Extensions/pgxntool 2.x.
@coderabbitai

coderabbitai Bot commented May 27, 2026 •
edited
Loading

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 87700535-cbde-4a9d-bac8-b10180d2601c

📥 Commits

Reviewing files that changed from the base of the PR and between d191ef1 and 259e663.

📒 Files selected for processing (1)
  • pgxntool/base.mk
📝 Walkthrough

Walkthrough

The PR modifies a single makefile variable assignment in pgxntool/base.mk. When Asciidoc is available, the DOCS variable is now rebuilt as a sorted de-duplicated union of existing entries and generated Asciidoc HTML targets, using filter-out to prevent duplicate targets. This replaces the prior unconditional append behavior that could introduce duplicate HTML files in the documentation build target list.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

A makefile once doubled its HTML with haste,
Now filters and sorts—no duplicates to waste.
One line of logic, so neat and so clean,
Deduplication, the best we've seen! 🐰✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title directly and concisely describes the main fix: preventing duplicate DOCS entries when asciidoctor is present, which matches the core change in the changeset.
Description check ✅ Passed The description is well-related to the changeset, explaining the problem (duplicate HTML paths causing install failures), the root cause, the fix applied, and includes a reproducer scenario.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-docs-duplicate-html

Comment @coderabbitai help to get the list of available commands and usage tips.

@jnasbyupgrade

jnasbyupgrade commented May 27, 2026

Copy link
Copy Markdown
Author

The correct fix is to update pgxntool, not hack it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jnasbyupgrade @claude